Reindex replacements spans for completions to be 0-indexed #12332
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So are we inconsistent with this across APIs? Seems like if editors are handling the positions in other APIs as 1-based, they should handle this also. Can you check the VS Code code to see if they are adjusting differently? |
|
Fixed on the vscode side in microsoft/vscode#15679 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #12165, and possibly #11462.
Module and triple-slash-reference completions were added in PR #9353. They are the only completions
we provided which offer
replacementSpan. The protocol the server uses to communicate with clients expects the spans in completion requests to be 0-indexed, but we were offering 1-indexed spans (consistent with spans for other types of requests such as quickinfo). This PR changes the server to behave as the editors (VS and VSCode) expect.In order to prevent an issue like this from arising again, we should write integration tests in VS Code / VS to exercise the default spans that the editors offer.